-
Notifications
You must be signed in to change notification settings - Fork 230
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update theme preview scripts and screenshots #909
Conversation
Thought: I'm wondering if Asciinema could be used to create smaller, more consistent/reproducible theme previews. |
Nice work!
I didn't know that was a thing. Does that work on all platforms too (mac/linux/windows)?
I'd be open to a more compact way of showing this. I once thought markdown with html tags indicating colors would do it, but I never pursued it. I'd only be open to asciinema you could view the preview inline without clicking on it or taking you to another site. |
That's a good question. I was thinking Windows-only, but since PowerShell and .Net are cross-platform, there's a chance it could work (with slight modifications). That said, making both of those be dependencies for generating previews might be a bit overkill.
As far as I know, that's completely possible. I'll try a quick single-theme prototype and see how it looks. |
Also, I'm not clear on the API here. How does this stuff work? I like the record view vs a table view in the screenshots. |
it's better than before so let's go ahead and land it. if we want to do asciinema we can in another pr. thanks. |
Seems like background, foreground, and cursor are a little bit weird. I'm not sure they're right after looking at the screenshots. So, we can't have background text be the color of the background or it would be invisible. I'm wondering how we can represent this better? How do other theme sites represent the background? Also, this isn't a criticism of anything you've done but looking at the nushell value colors like int, float, string, etc. The themes all stink. In my theme, most of these are different colors so I can easily differentiate them in a table. I wonder if there's a certain pattern we could apply based on the input where we could color every nushell value with a slightly different color? or maybe just have a rainbow setting that does this. Not sure what the right direction is. |
If you mean the module, it's basically just the old So: > # Old
> source preview-screenshot-script.nu
> preview_screenshot_small
> # New
> use .. *
> # Or, if using Nupm or the nu_scripts is in the library path then
> use themes *
> preview screenshot small
Well I was going to say the same to you! It's still pretty much the same table that you came up with in #519 to bucket the record into three columns. It's just that I prettied it up by chopping off the header (manually, via regex) and suppressed the indices. I also think the
I was a bit surprised at
Ah yeah, forgot to mention that change. Might could invert the foreground/background there? I'm flexible. My main goal was just to make the information visible, even if I wasn't representing the actual color.
Agreed - I had the same thought while looking at it. The actual shapes do have independent colors, but the value representations do not. I've often thought that |
Nope. When I created this preview, I was trying to find a way to represent the colors so that people know what it looks like without installing a theme. In my comment, I was specifically talking about fg, bg, cursor colors. Now that I think about it, fg is probably ok since it should be colored the same as the line. Like I said, we can't make bg text the color of the background because it would then be invisible. Maybe inverting the background would be ok. Not sure, I'd have to see a few. Or maybe draw the bg text with the fg as the bg color and the bg text as the fg color. (confusing for me to say, lol). Just trying to think of a way to make bg look more visible. |
I just noticed that the PNG files generated are more than 100k bigger as the previous ones. We should probably compress them better. I'm wondering if we could implement this CI to make sure they're always compressed? https://github.com/nushell/nushell.github.io/blob/main/.github/workflows/image-actions.yml |
Ah, I didn't compare before-and-after. I did try with Jpeg, thinking those would be smaller, but the PowerShell/.Net representation from the clipboard capture was even larger! I'm working on seeing how the Asciinema versions work now - It will auto-create a PNG that links to the recording, so let me see how large that is. |
I added the ci anyway |
Lots of changes - Pretty every change needed to be made before updating the preview screenshots, so they all end up in the same PR here:
preview-generate-screenshots.nu
now generates the previews and screenshots all in one pass rather than requiring a separate file.Adds a new method for generating screenshots using PowerShell with no third-party application required. Created new function for choosing the method that is used to generate screenshots.
Fixes background color on
nushell-dark
andnushell-light
themes.Fixes bug where
cursor
setting (and perhaps one other) was getting dropped from the preview table (math ceil
was needed rather thanmath floor
)Fixes
cellpath
setting - Was changed tocell-path
a while back but never updated in the themes. All themes and their previews were regenerated. Custom themes were manually adjusted.Important: I did not update
make.nu
to fix this (yet) since @amtoine has a open PR against it that has not been merged yet. Easier to either put that change in that PR or do it after that one has been merged.Adds
mod.nu
in the root of thethemes
module. Moves some of the previous script commands such aspreview_terminal
,preview_theme_small
, etc. over to the module. Makes for cleaner usage.Note: The completion functions in the scripts have been broken for several months. I did not fix this at this point.
preview_theme_small
(and others) renamed to use spaces in command name, so it is nowpreview theme small
.Fixes bug of my own making that prevented several custom themes from working.
Refactors the "list to columns" code to use
group
(will need to be updated tochunks
in 0.96)Suppresses the indices column in the preview table
Removes the column headers entirely since they don't serve any purpose in this type of "columnar table"